Skip to content

Comments

treewide: fix and improve some srcs#391262

Merged
philiptaron merged 5 commits intoNixOS:masterfrom
oxij:tree/fix-sources
Apr 5, 2025
Merged

treewide: fix and improve some srcs#391262
philiptaron merged 5 commits intoNixOS:masterfrom
oxij:tree/fix-sources

Conversation

@oxij
Copy link
Member

@oxij oxij commented Mar 19, 2025

This fixes some source-related build failures I noticed while building nixpkgs without using cache.nixos.org, and then adds some related improvements on top, to make these particular issues less likely in the future.

Things done

  • Built on platform(s)
    • x86_64-linux
  • With
    • sandbox = true
  • This should be a noop, since no hashes change.
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) labels Mar 19, 2025
@nix-owners nix-owners bot requested a review from philiptaron March 19, 2025 11:58
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 19, 2025
drupol
drupol reviewed Mar 19, 2025
View reviewed changes
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about replacing sha256 with hash where it is relevant too then ?

@oxij
Copy link
Member Author

oxij commented Mar 19, 2025

Well, I'd say it's out of scope of this, since this PR specifically on purpose does not touch any hashes.

@drupol
Copy link
Contributor

drupol commented Mar 19, 2025 •
edited
Loading

Just replacing the attribute, not its value, and only where it makes sense. And this is in the scope of "improving some src" since the attribute sha256 is part of the srcs.

e.g.: in pkgs/by-name/m1/m17n_db/package.nix

@oxij
Copy link
Member Author

oxij commented Mar 19, 2025

Done for all base64 hashes.

antonmosich
antonmosich reviewed Mar 19, 2025
View reviewed changes
@oxij oxij force-pushed the tree/fix-sources branch from 5dbfa2b to 616ca0c Compare March 19, 2025 18:20
@oxij
Copy link
Member Author

oxij commented Mar 20, 2025 via email

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 20, 2025
@nixos-discourse
Copy link

nixos-discourse commented Mar 21, 2025

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2314

@oxij oxij force-pushed the tree/fix-sources branch from 616ca0c to c403ab6 Compare March 23, 2025 13:24
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@oxij oxij force-pushed the tree/fix-sources branch from c403ab6 to 4c9993c Compare April 2, 2025 18:15
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@oxij
Copy link
Member Author

oxij commented Apr 2, 2025

Rebased to fix conflicts.

@oxij
Copy link
Member Author

oxij commented Apr 4, 2025

Darwin checks are stuck. But maybe this is fine to merge anyway?

@drupol
Copy link
Contributor

drupol commented Apr 4, 2025

Nope, all the tests are failing, I would not merge this at the moment.

@oxij
Copy link
Member Author

oxij commented Apr 4, 2025

Well xmlsec test fails, but this PR is a noop, I fail to see how this could be at fault.

philiptaron
philiptaron approved these changes Apr 5, 2025
View reviewed changes
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through and validated every single change. This is good to merge.

@philiptaron philiptaron merged commit 0a5ad4c into NixOS:master Apr 5, 2025
26 of 27 checks passed
@oxij
Copy link
Member Author

oxij commented Apr 6, 2025

Thanks @philiptaron!

@philiptaron philiptaron mentioned this pull request May 20, 2025
Open
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@drupol drupol drupol approved these changes

@philiptaron philiptaron philiptaron approved these changes

+2 more reviewers

@mmlb mmlb mmlb left review comments

@antonmosich antonmosich antonmosich left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@oxij @drupol @nixos-discourse @philiptaron @mmlb @antonmosich @wegank